Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Do not show notify on return #101

Merged
merged 1 commit into from
Jun 5, 2023
Merged

Fix: Do not show notify on return #101

merged 1 commit into from
Jun 5, 2023

Conversation

oliverfoster
Copy link
Member

fixes #100
fixes #94

Fix

  • Incorrect enumeration comparison causing feedback to inadvertently show

@oliverfoster oliverfoster self-assigned this Jun 1, 2023
This was referenced Jun 1, 2023
Closed
Closed
swashbuck
swashbuck approved these changes Jun 1, 2023
View reviewed changes
Copy link

@swashbuck swashbuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Tested with both "notify" and "inline" Tutor style, and it's working.

cahirodoherty-learningpool
cahirodoherty-learningpool approved these changes Jun 2, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

@joe-allen-89 joe-allen-89 merged commit 3e79541 into master Jun 5, 2023
@joe-allen-89 joe-allen-89 deleted the issue/100 branch June 5, 2023 10:33
github-actions bot pushed a commit that referenced this pull request Jun 5, 2023
@semantic-release-bot
Chore(release): 4.5.8 [skip ci]
f38d340 
## [4.5.8](v4.5.7...v4.5.8) (2023-06-05)

### Fix

* Do not show notify on return (#101) ([3e79541](3e79541)), closes [#101](#101)

### Upgrade

* Bump yaml and semantic-release (#93) ([723a352](723a352)), closes [#93](#93)
@github-actions
Copy link

github-actions bot commented Jun 5, 2023

🎉 This PR is included in version 4.5.8 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@Dougieladd
Copy link

Dougieladd commented Jun 14, 2023 •
edited
Loading

This is still an issue for feedback items that show in modals. I'm no coder... I'm just trying to find a solution to the problem. Currently, I have a bunch of MCQ's with feedback (showing in modals). After answering each question, on return to the page, all of the modal feedback windows are still showing.

@Dougieladd
Copy link

I have version 4.5.8

@oliverfoster
Copy link
Member Author

oliverfoster commented Jun 14, 2023 •
edited
Loading

@Dougieladd

I need to know how tutor is configured on your course.json and/or on your questions.
Some screen shots would be good also.

This code says that the feedback will show, only when _canShowFeedback is true, _isSubmitted is true and the tutor type is set to inline:

adapt-contrib-tutor/js/adapt-contrib-tutor.js

Lines 31 to 32 in ffc89fc

const shouldShowFeedback = (model.get('_canShowFeedback') && model.get('_isSubmitted') && type === TUTOR_TYPE.INLINE.asString);
if (!shouldShowFeedback) return;

Which obviously doesn't fit the course you've described, as you're using notify not inline, so they shouldn't be forced to show.

Either, there is a different bug, or you've not compiled the course correctly, or you've not updated the code correctly.

@Dougieladd
Copy link

Thanks... yeah I noticed "INLINE" in the code... but this is not possible for the course I'm doing... What do you need from me exactly? I can show you the issue I see (with modals), but what specifically would you like from me? And thank you for your swift reply... that's very helpful.

@Dougieladd
Copy link

This is how the MCQ's look... (There's 8 of them).
image

And this is the result on returning to the page... I've closed a few of them so you can see they are all on top of each other...
image

Let me know if there's something specific you'd like from me... I'm using the authoring tool. And these shots are on PREVIEW... but the same thing happens when I publish and test on SCORMCLOUD or our LMS

@oliverfoster
Copy link
Member Author

oliverfoster commented Jun 14, 2023 •
edited
Loading

Can you export the course and send over the zip? I want to check the version numbers and code. It sounds very much like you don't have the code for the current plugin version. Have you force rebuilt?

@Dougieladd
Copy link

I'm not sure I'd be allowed to do that... as it's in build. I've not tried the force rebuild though. I can try that. Are there any files in the published zip I could share?

@oliverfoster
Copy link
Member Author

oliverfoster commented Jun 14, 2023 •
edited
Loading

bug

This is a standard vanilla course.

I'm not sure I'd be allowed to do that... as it's in build. I've not tried the force rebuild though. I can try that. Are there any files in the published zip I could share?

The two files I referenced above are the only important ones:

course.json
src/extensions/adapt-contrib-tutor/js/adapt-contrib-tutor.js

@Dougieladd
Copy link

Dougieladd commented Jun 14, 2023 •
edited
Loading

We updated the framework to the latest version last week (or the week before) and I updated the plugin on Monday. I'll just check the force rebuild.

@Dougieladd
Copy link

Hi Oliver, the force rebuild seems to have done the trick for me. You're a genius! I'm just checking with other members of the team (they're working on other client courses with the same issue) to see if it has sorted them out too. But it looks good so far. Thank you for your swift reply and help.

@oliverfoster
Copy link
Member Author

Awesome. Glad you fixed it!

@Dougieladd
Copy link

Dougieladd commented Jun 14, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swashbuck swashbuck swashbuck approved these changes

@cahirodoherty-learningpool cahirodoherty-learningpool cahirodoherty-learningpool approved these changes

@joe-allen-89 joe-allen-89 joe-allen-89 approved these changes

Assignees

@oliverfoster oliverfoster

Labels
bug released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Question component notify popup Question component feedback popups opening up on page load
5 participants
@oliverfoster @Dougieladd @swashbuck @cahirodoherty-learningpool @joe-allen-89